Skip to content

chore(cli): clean up unused CodeQL declarations - #4906

Merged
cv merged 5 commits into
mainfrom
codex/cleanup-codeql-unused
Jun 7, 2026
Merged

chore(cli): clean up unused CodeQL declarations#4906
cv merged 5 commits into
mainfrom
codex/cleanup-codeql-unused

Conversation

@cv

@cv cv commented Jun 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Removes unused declarations reported by CodeQL's js/unused-local-variable rule across onboarding, inference helpers, scripts, and tests. Also stabilizes two test fixtures that blocked local full-hook verification under a restrictive umask and parallel temp-dir usage.

Changes

  • Removed unused imports, constants, functions, and destructured bindings from src/lib/onboard.ts, inference helpers, the source-shape scanner, and affected tests.
  • Ratcheted legacy test-file size budgets after shrinking oversized test files.
  • Made permission and temp-dir assertions deterministic without changing production behavior.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

Summary by CodeRabbit

  • Tests

    • Updated test file imports and removed unused dependencies across multiple test suites.
    • Enhanced permission-handling tests for sandbox configuration validation scenarios.
    • Refactored test fixtures and improved test assertion clarity.
  • Chores

    • Reduced test file size budgets to reflect code optimization.
    • Removed unused internal helper functions and imports throughout the codebase.

@cv cv self-assigned this Jun 7, 2026
@coderabbitai

coderabbitai Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 57542f6c-7581-4bf9-9bdc-1bfc6907bbaa

📥 Commits

Reviewing files that changed from the base of the PR and between 9dd7218 and 1ffa8a7.

📒 Files selected for processing (17)
  • ci/test-file-size-budget.json
  • scripts/find-source-shape-tests.ts
  • src/lib/cli/command-display-metadata.test.ts
  • src/lib/dashboard-url-command.test.ts
  • src/lib/inference/ollama/proxy.ts
  • src/lib/inference/onboard-probes.ts
  • src/lib/onboard.ts
  • src/lib/onboard/config-sync.test.ts
  • src/lib/state/config-io.test.ts
  • test/cli-oclif-compatibility.test.ts
  • test/e2e-scenario/framework-tests/e2e-scenario-registry.test.ts
  • test/e2e/brev-e2e.test.ts
  • test/gateway-state-reconcile-2276.test.ts
  • test/nemoclaw-start.test.ts
  • test/policies.test.ts
  • test/sandbox-provisioning.test.ts
  • test/skills-frontmatter.test.ts
💤 Files with no reviewable changes (10)
  • test/e2e/brev-e2e.test.ts
  • test/cli-oclif-compatibility.test.ts
  • src/lib/inference/onboard-probes.ts
  • test/gateway-state-reconcile-2276.test.ts
  • test/sandbox-provisioning.test.ts
  • test/nemoclaw-start.test.ts
  • scripts/find-source-shape-tests.ts
  • src/lib/inference/ollama/proxy.ts
  • src/lib/cli/command-display-metadata.test.ts
  • test/skills-frontmatter.test.ts

📝 Walkthrough

Walkthrough

This PR performs a comprehensive code cleanup across the NemoClaw codebase, removing unused imports, helper functions, and constants from source and test files. It includes a significant refactoring of wiring in src/lib/onboard.ts, enhances permission-safety test setups with explicit chmod calls, improves test assertions using spy-based verification, and updates test size budgets accordingly.

Changes

Code Cleanup and Test Enhancement

Layer / File(s) Summary
Script and module-level import/helper removals
scripts/find-source-shape-tests.ts, src/lib/inference/ollama/proxy.ts, src/lib/inference/onboard-probes.ts
Unused helper functions and imports are removed from build scripts and inference modules. find-source-shape-tests.ts removes isSourceTextLikeName and isTextDerivation helpers; proxy.ts removes unused http import and the clearPersistedProxyPid function; onboard-probes.ts removes unused shouldForceCompletionsApi import.
Onboard.ts import and helper restructuring
src/lib/onboard.ts
Refactored destructured imports and removed unused helpers across multiple subsystems: runner/name-validation wiring, local-inference Ollama bindings, Hermes auth helpers, sandbox lifecycle utilities, web-search flow helpers, probe endpoint imports, and inference provider recovery reads narrowed to recorded-only. Removed local variable resumeSandboxNameForGpu from GPU-resume logic.
Test file import cleanup
src/lib/cli/command-display-metadata.test.ts, src/lib/dashboard-url-command.test.ts, test/cli-oclif-compatibility.test.ts, test/e2e/brev-e2e.test.ts, test/e2e-scenario/framework-tests/e2e-scenario-registry.test.ts, test/gateway-state-reconcile-2276.test.ts, test/sandbox-provisioning.test.ts, test/skills-frontmatter.test.ts
Removed unused imports (fs, getRegisteredOclifCommandsMetadata, listScenarios, DashboardUrlCommandError), fixture constants (SANDBOX_GET_TRANSPORT_ERROR, Hermes policy constants), and unused loop variables (isSkill).
Permission and symlink safety test setup improvements
src/lib/state/config-io.test.ts, src/lib/onboard/config-sync.test.ts
Added explicit fs.chmodSync(..., 0o644) and fs.chmodSync(..., 0o755) setup calls before test assertions to ensure consistent initial file permission states in symlink-safety and permission-healing scenarios.
Test assertion methodology updates
test/nemoclaw-start.test.ts, test/policies.test.ts
Removed configureGuardBlock helper function from nemoclaw-start.test.ts. Updated policies.test.ts to replace directory-count snapshots at process.exit time with a spy on fs.mkdtempSync filtered by temp directory prefix, validating absence of matching calls before OpenShell resolvability check.
Test size budget alignment
ci/test-file-size-budget.json
Updated legacyMaxLines budget values: reduced test/nemoclaw-start.test.ts from 5319 to 5310 and test/policies.test.ts from 3147 to 3145 to reflect line count changes from test refactoring.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Possibly related PRs

  • NVIDIA/NemoClaw#4628: Updates to permission-healing test setup in src/lib/state/config-io.test.ts directly correspond to symlink-safety and chmod behavior covered in that PR.
  • NVIDIA/NemoClaw#4905: Both PRs modify ci/test-file-size-budget.json—the retrieved PR introduces the budget guard, and this PR adjusts entries for tests with changed line counts.

Suggested labels

area: cli, NemoClaw CLI, Sandbox


Suggested reviewers

  • prekshivyas
  • cjagwani

🐰 Hops through the codebase with glee,
Removing unused imports, setting helpers free!
With chmod guards and spy-checks so fine,
The tests now behave—everything's in line!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore(cli): clean up unused CodeQL declarations' directly and specifically describes the main change: removing unused declarations flagged by CodeQL across the codebase.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/cleanup-codeql-unused

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: ollama-proxy-e2e, strict-tool-call-probe-e2e, onboard-inference-smoke-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E

  • None. No merge-blocking E2E is recommended: the production-file edits shown are unused import/unused symbol cleanup in critical onboarding and inference modules, while the remaining changes are tests or CI tooling. Build/type/unit coverage should catch accidental symbol issues, and there is no apparent runtime behavior change to installer, onboarding flow, sandbox lifecycle, credentials, network policy, or inference routing.

Optional E2E

  • ollama-proxy-e2e (medium (~15 minutes; installs Ollama and pulls a small model)): Optional confidence because src/lib/inference/ollama/proxy.ts was touched. The diff looks like unused-code cleanup, but this existing E2E validates real Ollama auth proxy token auth, inference through the proxy, persistence, recovery, and container reachability.
  • strict-tool-call-probe-e2e (low-medium): Optional confidence because src/lib/inference/onboard-probes.ts was touched. This hermetic regression E2E validates strict Chat Completions tool-call probe payload shape and retry behavior without needing GPU/Ollama infrastructure.
  • onboard-inference-smoke-e2e (low-medium): Optional confidence because src/lib/onboard.ts and inference probe wiring were touched. This regression E2E checks that onboarding does not report success until the configured inference route serves a real request.

New E2E recommendations

  • None.

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw, gpu-repo-local-ollama-openclaw
Optional scenario E2E: ubuntu-repo-cloud-hermes, ubuntu-repo-openai-compatible-openclaw

Dispatch required scenario E2E:

  • gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw
  • gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=gpu-repo-local-ollama-openclaw

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required scenario E2E

  • ubuntu-repo-cloud-openclaw: Core onboarding and inference probe code changed in src/lib/onboard.ts and src/lib/inference/onboard-probes.ts; this is the primary Ubuntu repo-current cloud OpenClaw scenario covering baseline onboarding, sandbox creation, cloud inference, and credentials.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw
  • gpu-repo-local-ollama-openclaw: src/lib/inference/ollama/proxy.ts changed; the GPU local Ollama scenario is the only dispatchable scenario route that exercises the Ollama auth proxy/local Ollama onboarding surface.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=gpu-repo-local-ollama-openclaw

Optional scenario E2E

  • ubuntu-repo-cloud-hermes: Optional adjacent coverage for the same repo-current cloud onboarding/inference path with the Hermes agent; useful because src/lib/onboard.ts is shared across agents, but OpenClaw is the primary target.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-hermes
  • ubuntu-repo-openai-compatible-openclaw: Optional adjacent coverage for OpenAI-compatible onboarding/probe selection paths touched by src/lib/inference/onboard-probes.ts; not primary because the diff appears to remove unused imports rather than alter probe behavior.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-openai-compatible-openclaw

Relevant changed files

  • src/lib/inference/ollama/proxy.ts
  • src/lib/inference/onboard-probes.ts
  • src/lib/onboard.ts
  • test/e2e-scenario/framework-tests/e2e-scenario-registry.test.ts

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Top item: No actionable code-review findings

Consider writing more tests for
  • **Runtime validation** — Verify the source-shape scanner still reports production source reads derived through helper functions and source-tree enumeration after removing dead scanner helpers.. The patch touches onboarding/inference and infrastructure-adjacent files, but the actual production changes are unused-declaration removals. Existing focused tests cover scanner behavior, permission repair boundaries, and policy temp-dir ordering; runtime validation is useful for confidence but no specific missing test blocks this cleanup.
  • **Runtime validation** — Verify `policy applyPreset` exits on an unresolved OpenShell binary before requesting a `nemoclaw-policy-*` temp directory.. The patch touches onboarding/inference and infrastructure-adjacent files, but the actual production changes are unused-declaration removals. Existing focused tests cover scanner behavior, permission repair boundaries, and policy temp-dir ordering; runtime validation is useful for confidence but no specific missing test blocks this cleanup.
  • **Runtime validation** — Verify config permission tests preserve the symlink target mode and heal only host `~/.nemoclaw` siblings under a restrictive umask.. The patch touches onboarding/inference and infrastructure-adjacent files, but the actual production changes are unused-declaration removals. Existing focused tests cover scanner behavior, permission repair boundaries, and policy temp-dir ordering; runtime validation is useful for confidence but no specific missing test blocks this cleanup.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@cv
cv merged commit b70816a into main Jun 7, 2026
40 checks passed
@cv
cv deleted the codex/cleanup-codeql-unused branch June 7, 2026 12:26
@cv cv added the v0.0.61 label Jun 7, 2026
@wscurran wscurran added the chore Build, CI, dependency, or tooling maintenance label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Build, CI, dependency, or tooling maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants